Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tyler thome GitHub handle 7175 #7346

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Add tyler thome GitHub handle 7175 #7346

merged 3 commits into from
Aug 27, 2024

Conversation

ihop-56
Copy link
Member

@ihop-56 ihop-56 commented Aug 23, 2024

Fixes #7175

What changes did you make?

  • Added the github-handle: field under name: Tyler Thome in the _projects/home-unite-us.md file. This field was added without a value as per the issue's instructions.

Why did you make the changes?

  • The change was made to prepare the project file for future enhancements, reducing redundancy by standardizing the GitHub handle field.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

No visual changes to the website.

@github-actions GitHub Actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b ihop-56-add-tyler-thome-github-handle-7175 gh-pages
git pull https://github.com/ihop-56/website.git add-tyler-thome-github-handle-7175

@github-actions github-actions bot added good first issue Good for newcomers role: front end Tasks for front end developers role: back end/devOps Tasks for back-end developers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) Status: Updated No blockers and update is ready for review size: 0.25pt Can be done in 0.5 to 1.5 hours labels Aug 23, 2024
@bexux bexux self-requested a review August 23, 2024 21:03
@bexux
Copy link
Member

bexux commented Aug 23, 2024

Review ETA: 8 PM pacific 8/24/24
Availability: 5-8 PM Friday, Noon - 8PM Saturday

bexux
bexux reviewed Aug 23, 2024
View reviewed changes
_projects/home-unite-us.md Outdated Show resolved Hide resolved
bexux
bexux requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@bexux bexux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very close to being done! Looks like you had the code correct on your first commit, but then the second commit, Changes before switching branches added in value that is not needed for this issue.

The PR description reads well, branching done correctly, and issue attached correctly. Just need the small change.

@bexux
Copy link
Member

bexux commented Aug 23, 2024

@ihop-56 I noticed you have a second PR - 7309 pushed up to address issue 7175. I assume this PR is in response to @codyyjxn's feedback to have the issue number in the branch name?

@ihop-56
Copy link
Member Author

ihop-56 commented Aug 23, 2024 via email

@terrencejihoonjung terrencejihoonjung requested review from terrencejihoonjung and removed request for terrencejihoonjung August 24, 2024 04:29
@bexux bexux mentioned this pull request Aug 25, 2024
Closed
@ihop-56
Copy link
Member Author

ihop-56 commented Aug 25, 2024

Hi @bexux,

I've made the requested changes:

  • Removed the value from github-handle for Tyler Thome as per your feedback.

Please review and let me know if any further adjustments are needed.

Thank you for your guidance!

@t-will-gillis
Copy link
Member

HI @ihop-56 please don't forget to re-request a review by selecting the chasing arrows next to the reviewer's handle when you make changes to address a reviewer's comments:
Screenshot 2024-08-25 083845

@codyyjxn codyyjxn self-requested a review August 25, 2024 21:33
codyyjxn
codyyjxn approved these changes Aug 25, 2024
View reviewed changes
Copy link
Member

@codyyjxn codyyjxn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice @ihop-56 this looks great

8alpreet
8alpreet approved these changes Aug 25, 2024
View reviewed changes
Copy link
Member

@8alpreet 8alpreet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ihop-56,

Thanks for making changes in accordance to feedback on this and the previous PR.

Opinion:

  • The PR looks good, but I would encourage you to do one last thing: squash the commits. I'm not sure if this is practice on this project, but merging three commits will pollute the the commit history unnecessarily.

Nitpick:

  • Not necessary, but you could capitalize "Tyler Thome" in the PR title.

I am approving, but urge you to squash the commits.

@t-will-gillis
Copy link
Member

Hey @8alpreet and @ihop-56 FYI the commits are squashed when we merge the PRs, thus we do not ask anyone to squash their commits. Devs can if they would like to, but since we 'squash and merge' it is not needed for the PR

bexux
bexux approved these changes Aug 26, 2024
View reviewed changes
Copy link
Member

@bexux bexux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you for making the changes.

@bexux bexux mentioned this pull request Aug 26, 2024
Open
@8alpreet
Copy link
Member

Hey @8alpreet and @ihop-56 FYI the commits are squashed when we merge the PRs, thus we do not ask anyone to squash their commits. Devs can if they would like to, but since we 'squash and merge' it is not needed for the PR

Got it, thanks @t-will-gillis!

@codyyjxn codyyjxn mentioned this pull request Aug 27, 2024
Open
kgold2018
kgold2018 approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@kgold2018 kgold2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ihop-56 ! Looks good! Thank you for making the changes.

@t-will-gillis t-will-gillis merged commit 3ab292d into hackforla:gh-pages Aug 27, 2024
3 checks passed
@ihop-56 ihop-56 mentioned this pull request Aug 30, 2024
Open
@8alpreet 8alpreet mentioned this pull request Sep 4, 2024
Open
@kgold2018 kgold2018 mentioned this pull request Sep 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codyyjxn codyyjxn codyyjxn approved these changes

@kgold2018 kgold2018 kgold2018 approved these changes

@8alpreet 8alpreet 8alpreet approved these changes

@bexux bexux bexux approved these changes

Assignees
No one assigned
Labels
good first issue Good for newcomers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) role: back end/devOps Tasks for back-end developers role: front end Tasks for front end developers size: 0.25pt Can be done in 0.5 to 1.5 hours Status: Updated No blockers and update is ready for review
Projects
P: HfLA Website: Project Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add github-handle for Tyler Thome in home-unite-us.md
6 participants
@ihop-56 @bexux @t-will-gillis @8alpreet @kgold2018 @codyyjxn